Skip to content

test: isolate missing-env reporting state (Fixes #500) - #501

Merged
Karthik Nadig (karthiknadig) merged 3 commits into
mainfrom
test/issue-500
Aug 6, 2026
Merged

test: isolate missing-env reporting state (Fixes #500)#501
Karthik Nadig (karthiknadig) merged 3 commits into
mainfrom
test/issue-500

Conversation

@karthiknadig

@karthiknadig Karthik Nadig (karthiknadig) commented Aug 5, 2026

Copy link
Copy Markdown
Member

Summary

  • inject missing-env reporting atomics into the state-machine helpers
  • keep production wrappers on the process-global state
  • give each unit test an independent local atomic instead of a shared mutex/global
  • prevent configure tests from racing or poisoning missing-env tests

Validation

  • 20 consecutive cargo test -p pet --bin pet --quiet runs
  • .\scripts\rust-precommit.ps1

Fixes #500

@karthiknadig
test: isolate missing-env reporting state (Fixes #500)
5e54ee1 
Inject the atomic state into state-machine helpers so parallel configure tests cannot corrupt missing-env reporting tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown

Performance Report (Linux) ➖

Metric PR (P50) PR (P95) Baseline (P50) Delta Change
Server Startup 1ms 1ms 1ms 0ms 0%
Full Refresh 57ms 189ms 55ms 2ms 0%

Results based on 10 iterations. P50 = median, P95 = 95th percentile.

Legend
  • 🚀 Significant speedup (>100ms faster)
  • ✅ Faster than baseline
  • ➖ No significant change
  • 🔺 Slower than baseline (>100ms)
  • ⚠️ Significant slowdown (>500ms)

@github-actions

github-actions Bot commented Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown

Performance Report (macOS)

Metric PR (P50) PR (P95) Baseline (P50) Delta
Server Startup 101ms 616ms 69ms 32ms
Full Refresh 172ms 30259ms 118ms 54ms

Results based on 10 iterations. P50 = median, P95 = 95th percentile.

Legend
  • 🚀 Significant speedup (>100ms faster)
  • ✅ Faster than baseline
  • ➖ No significant change
  • 🔺 Slower than baseline (>100ms)
  • ⚠️ Significant slowdown (>500ms)

@github-actions

github-actions Bot commented Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown

Test Coverage Report (Linux)

Metric Value
Current Coverage 80.6%
Base Branch Coverage 80.6%
Delta 0% ➖

Coverage unchanged.

@github-actions

github-actions Bot commented Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown

Performance Report (Windows) ➖

Metric PR (P50) PR (P95) Baseline (P50) Delta Change
Server Startup 9ms 12ms 9ms 0ms 0%
Full Refresh 165ms 567ms 164ms 1ms 0.6%

Results based on 10 iterations. P50 = median, P95 = 95th percentile.

Legend
  • 🚀 Significant speedup (>100ms faster)
  • ✅ Faster than baseline
  • ➖ No significant change
  • 🔺 Slower than baseline (>100ms)
  • ⚠️ Significant slowdown (>500ms)

@github-actions

github-actions Bot commented Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown

Test Coverage Report (Windows)

Metric Value
Current Coverage 77.24%
Base Branch Coverage 77.31%
Delta -0.07% ❌

Coverage decreased. Please add tests for new code.

Copilot AI approved these changes Aug 5, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Ready to approve

The changes align with the issue acceptance criteria and cleanly isolate test state while preserving production behavior via wrappers over the existing global atomic.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR reduces test flakiness around missing-environment reporting in pet::jsonrpc by extracting the missing-env reporting state-machine helpers to accept an injected AtomicU64, while keeping production behavior backed by the existing process-global atomic. Tests are updated to use per-test local atomics, removing the need for a shared test mutex and preventing cross-test interference with configure-related state changes.

Changes:

  • Added *_with_state variants for missing-env reporting helpers that accept an injected &AtomicU64, with thin production wrappers continuing to use MISSING_ENVS_REPORTING_STATE.
  • Removed the global MISSING_ENVS_TEST_LOCK and updated missing-env-reporting unit tests to use independent local AtomicU64 instances.
  • Adjusted the configure-reset test to validate the reset behavior using the injected state rather than process-global state.
File summaries
File Description
crates/pet/src/jsonrpc.rs Extracts missing-env reporting state-machine helpers to accept injected atomics and updates unit tests to avoid shared process-global state.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

@karthiknadig
Karthik Nadig (karthiknadig) marked this pull request as ready for review August 5, 2026 23:13
@karthiknadig
Karthik Nadig (karthiknadig) merged commit 8bd5fb4 into main Aug 6, 2026
37 checks passed
@karthiknadig
Karthik Nadig (karthiknadig) deleted the test/issue-500 branch August 6, 2026 04:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmitrivMS Dmitriy Vasyura (dmitrivMS) dmitrivMS approved these changes
@anthonykim1 Anthony Kim (anthonykim1) anthonykim1 approved these changes
Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Isolate missing-env reporting state tests

4 participants

@karthiknadig @dmitrivMS @anthonykim1